Skip to content

Make compiler requests safe to run repeatedly and in parallel - #8864

Open
cknitt wants to merge 7 commits into
masterfrom
compiler-request-isolation
Open

cknitt wants to merge 7 commits into
masterfrom
compiler-request-isolation

Conversation

@cknitt

@cknitt cknitt commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

This makes compiler requests safe to run repeatedly, and on several OCaml domains at once, inside one process. bsc keeps its behaviour. The OCaml build system (#8780) will use this to compile in-process instead of starting bsc for every file (#8781), which is faster than Rust rewatch. The change is split out of #8781 so it can be reviewed on its own.

Changes

  • Request-scoped state. The compiler's process-wide mutable state moves into domain-local, request-scoped state: Clflags, Js_config, warnings, environment and CRC caches, predefined types, Ident stamps, printers, diagnostics and backend caches. Almost all of it lives in Request_local keys:

    • every request starts with fresh values, as a new bsc process would;
    • the previous values are restored afterwards;
    • keys initialize in registration order.

    Only Ident stamps, the request context, output capture and the order-sensitive predefined types keep explicit handling. Clflags.reset is the single definition of bsc's defaults.

  • Shared driver. The driver moves from rescript_compiler_main.ml into the rescript_compiler_driver library. bsc runs through it, and an embedding process can call run_request with its own working directory and its own runner for external commands such as PPXs. A relative -bs-project-root is resolved against that directory.

  • Output capture. An embedded request's output is captured in memory, without temporary files. On Windows the captured text gets bsc's CRLF line endings, without doubling those a PPX already wrote. Output written before an escaping exception is kept and reported with it. The standalone bsc still writes directly to its stdout and stderr.

  • Fixes found along the way:

    • races and leaked state that a single process never hit: Js_analyzer's free-variable accumulator was shared, so Js_shake could drop a live function definition; also Parmatch.Conv's name counter, Ctype's abbreviation memo and levels, and the parser hooks for error messages;
    • a failing or interrupted PPX no longer leaves its temporary AST files behind, and they have absolute paths when the temporary directory is relative;
    • a failing PPX's stderr no longer shifts the indentation of the error that follows it;
    • readers of .cmj, .ast and PPX output close their file when decoding fails, writers of .ast, .cmj and .cmi close it when writing fails, and .cmi files are opened close-on-exec;
    • -e's temporary source file is absolute when the temporary directory is relative, and is removed when compiling it fails;
    • bsc -verbose prints a PPX's command before the PPX's own output again.

Cost for bsc

Request-scoped state adds a domain-local lookup per access. For the standalone bsc this is not measurable: in clean builds of tests/tests (629 modules) through Rust rewatch, 9 interleaved runs per binary, the minimum CPU time was 12.66 s on master and 12.70 s with this PR, and the medians 13.49 s and 13.37 s.

ThreadSanitizer

An OCaml 5.5.0 ThreadSanitizer build of the in-process build system reported no data races in compiler code, on clean builds of rewatch/testrepo (4 domains), tests/tests (2, 8 and 12 domains), the gentype example and the React PPX tests. Two reports appeared in some tests/tests runs, both inside the runtime's parallel minor GC (oldify_one against pool_allocate). With the shared Js_analyzer accumulator restored, the same build reports 91 races, all in Js_analyzer, so the tool does catch this class of bug.

Tests

tests/ounit_tests/ounit_compiler_request_tests.ml checks that each kind of request-scoped state is fresh in every request and isolated between consecutive and concurrent requests, including:

  • captured output and its line endings;
  • the abbreviation memo, and the type-node counter (through the .cmt);
  • generated names;
  • artifact readers closing their file when decoding fails;
  • the initialization order of Request_local keys.

🤖 Generated with Claude Code

@cknitt
cknitt force-pushed the compiler-request-isolation branch from 4239864 to 5b2d4a8 Compare October 14, 2026 12:21
@cknitt
cknitt marked this pull request as ready for review October 14, 2026 12:22
@codecov

codecov Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.84370% with 336 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.94%. Comparing base (7957c10) to head (4acb29d).

Files with missing lines Patch % Lines
compiler/bsc/rescript_compiler_driver.ml 81.22% 52 Missing ⚠️
compiler/ml/ctype.ml 82.06% 47 Missing ⚠️
compiler/ml/printtyp.ml 60.34% 46 Missing ⚠️
tests/ounit_tests/ounit_compiler_request_tests.ml 97.68% 22 Missing ⚠️
compiler/ext/compiler_request_output.ml 76.54% 19 Missing ⚠️
compiler/ml/env.ml 83.47% 19 Missing ⚠️
compiler/gentype/debug.ml 56.09% 18 Missing ⚠️
compiler/ml/typecore.ml 91.73% 10 Missing ⚠️
compiler/core/js_implementation.ml 79.06% 9 Missing ⚠️
compiler/ml/cmi_format.ml 63.63% 8 Missing ⚠️
... and 46 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8864      +/-   ##
==========================================
+ Coverage   80.51%   80.94%   +0.43%     
==========================================
  Files         447      453       +6     
  Lines       61060    62839    +1779     
==========================================
+ Hits        49162    50865    +1703     
- Misses      11898    11974      +76     
Files with missing lines Coverage Δ
analysis/src/module_resolution.ml 18.18% <100.00%> (ø)
compiler/common/js_config.ml 81.81% <100.00%> (+6.81%) ⬆️
compiler/core/bs_conditional_initial.ml 100.00% <ø> (ø)
compiler/core/config_util.ml 93.75% <100.00%> (-6.25%) ⬇️
compiler/core/ir_diagnostics.ml 92.59% <100.00%> (ø)
compiler/core/js_analyzer.ml 82.91% <100.00%> (-0.32%) ⬇️
compiler/core/js_dump.ml 92.67% <100.00%> (+<0.01%) ⬆️
compiler/core/js_dump_program.ml 98.46% <100.00%> (ø)
compiler/core/lam_beta_reduce_util.ml 96.87% <100.00%> (-0.35%) ⬇️
compiler/core/lam_compile_env.ml 86.53% <100.00%> (+2.09%) ⬆️
... and 105 more

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new

pkg-pr-new Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@8864

@rescript/belt

npm i https://pkg.pr.new/@rescript/belt@8864

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@8864

@rescript/darwin-x64

npm i https://pkg.pr.new/@rescript/darwin-x64@8864

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@8864

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@8864

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@8864

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@8864

commit: 4acb29d

@github-actions

Copy link
Copy Markdown

@cknitt

cknitt commented Oct 14, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-14T18:15:54.964408Z 1d81eef Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5b2d4a83bd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread compiler/bsc/rescript_compiler_driver.ml Outdated
cknitt and others added 2 commits October 14, 2026 14:55
Process-wide compiler state becomes request-scoped, so one process can run many compiler requests, also on several domains at once. bsc keeps its behavior; the OCaml build system will compile in-process with this.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WZbCuRQ1Tn31bnKzLj5e72
Signed-off-by: Christoph Knittel <ck@cca.io>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WZbCuRQ1Tn31bnKzLj5e72
Signed-off-by: Christoph Knittel <ck@cca.io>
@cknitt
cknitt force-pushed the compiler-request-isolation branch from 5b2d4a8 to 2c177e8 Compare October 14, 2026 15:09
@cknitt
cknitt added this pull request to stack #8865 October 14, 2026 15:52
@cknitt

cknitt commented Oct 14, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2c177e8fa3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread compiler/bsc/rescript_compiler_driver.ml Outdated
cknitt and others added 4 commits October 14, 2026 17:40
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WZbCuRQ1Tn31bnKzLj5e72
Signed-off-by: Christoph Knittel <ck@cca.io>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WZbCuRQ1Tn31bnKzLj5e72
Signed-off-by: Christoph Knittel <ck@cca.io>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WZbCuRQ1Tn31bnKzLj5e72
Signed-off-by: Christoph Knittel <ck@cca.io>
Also tests the writer and capture fixes before it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WZbCuRQ1Tn31bnKzLj5e72
Signed-off-by: Christoph Knittel <ck@cca.io>
@cknitt

cknitt commented Oct 14, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1d81eef0aa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread compiler/bsc/rescript_compiler_driver.ml
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WZbCuRQ1Tn31bnKzLj5e72
Signed-off-by: Christoph Knittel <ck@cca.io>
@cknitt
cknitt requested a review from cristianoc October 15, 2026 04:15

@cristianoc cristianoc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fantastic
And zero cost

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants